[webview_flutter] Enable warnings-as-errors on Android #3356
Conversation
| * @return Whether any cookies were removed. | ||
| */ | ||
| @SuppressWarnings("deprecation") | ||
| private boolean removeCookiesLegacy(CookieManager cookieManager) { |
There was a problem hiding this comment.
Nit: Consider naming this removeAllCookiesPreL.
I looked for google documentation but basically "legacy" and "new" and a few other words make sense now but lose their meaning over time. PreL sets a clear expectation and allows it to live along other split methods that can handle future deprecations.
There was a problem hiding this comment.
Hah, this is actually why I asked in Discord about numeric constants vs letters, but I named it "Legacy" in the meantime and then forgot to come back and change it after you answered. I'll change it now.
…d forceCodeForRefreshToken parameter (flutter#3356)
| @@ -29,4 +29,4 @@ dev_dependencies: | |||
| flutter_test: | |||
| sdk: flutter | |||
| mockito: ^5.3.2 | |||
| pigeon: ^4.2.14 | |||
| pigeon: ^9.0.4 | |||
There was a problem hiding this comment.
Since you are updating the pigeon version, could you try and regenerate the mocks as well. I tried raising the pigeon version too, but it seems the new analyzer was causing mockito to generate bad mocks. I want to see if you run into the same problem or if it was specific to me.
There was a problem hiding this comment.
It's not just you; the Future<Offset> and Future<Size> methods are wrong. I'm trying to make a reduced test case to file an issue, but a trivial class with a Future<Offset> works, so it seems like there's something more complicated going on.
There was a problem hiding this comment.
No luck so far. Maybe we should just file it with webview_flutter as-is, and hope someone who knows the internals better can figure out what's going on more easily that I can figure out what the minimal repro is.
How do you want to handle this in the meantime? Hold this PR, or manually fix it in the mocks with a TODO and issue link?
There was a problem hiding this comment.
Would it be possible just to revert the version bump to pigeon? Then this PR should be good.
There was a problem hiding this comment.
+1 to submitting an issue with webview_flutter as-is. I think we can just give them the android_webview_test.dart with the android_webview.dart and say that the mockito outputs are incorrect. I can also handle making the issue if you would like.
There was a problem hiding this comment.
I filed dart-lang/mockito#612, it turns out it's a dependency issue that doesn't reproduce if you clean or upgrade.
There was a problem hiding this comment.
Thanks for filing it!
stuartmorgan-g
added
the
autosubmit
* main: (3910 commits) [various] Align Flutter and Dart SDK constraints (flutter#3349) Roll Flutter from c590086 to f2f8005 (14 revisions) (flutter#3373) [webview_flutter] Enable warnings-as-errors on Android (flutter#3356) [ci] Increase Android platform test sharding (flutter#3365) Roll Flutter from f032a4d to c590086 (69 revisions) (flutter#3366) [Espresso] Update truth package to 1.1.3 (flutter#3358) [google_maps] Relax the Android renderer requset test (flutter#3364) [pigeon] Only check generated files on master (flutter#3357) [webview]: Bump androidx.webkit:webkit from 1.5.0 to 1.6.0 in /packages/webview_flutter/webview_flutter_android/android (flutter#3243) [ci+various] Partially enable javac warning checks (flutter#3293) [webview_flutter] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3336) [local_auth] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3335) [google_sign_in] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3330) [google_maps_flutter] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3329) [video_player] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3328) [file_selector] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3325) [go_router_builder] Fix the example for default values in the README (flutter#3231) Update annotation and espresso dependencies (flutter#3271) [tool] Provide a --base-branch flag (flutter#3322) [image_picker_android] Adds Android 13 photo picker functionality (flutter#3267) ...
[webview_flutter] Enable warnings-as-errors on Android
Enables the javac lint and werror flags for
webview_flutter_android, fixing the resulting errors.This includes a Pigeon roll since a number of warnings were from Pigeon-generated code, which has since been fixed in Pigeon.
Part of flutter/flutter#91868
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).